Repository navigation
Feature/db size - #16
Conversation
| */ | ||
| public function size( $args, $assoc_args ) { | ||
|
|
||
| WP_CLI::get_runner()-> load_wordpress(); |
There was a problem hiding this comment.
You can suppress this with @ for now.
| * ## OPTIONS | ||
| * | ||
| * [--db-only] | ||
| * : Only display the database size without including the tables. |
There was a problem hiding this comment.
For syntax, what do you think about this?
$ wp db size
# Prints database name and size as ASCII table
$ wp db size --tables
# Prints each table name and size as ASCII table
This would differ a little bit from what you have in that the default would be only the database size, and then you'd use the --tables flag to list all tables.
| * - table | ||
| * - csv | ||
| * - json | ||
| * - bytes, only works when combined with --db-only |
There was a problem hiding this comment.
I think this might work better as a --size_format=<format> argument that accepts one of b, kb, or mb. The default could be kb.
| "db import", | ||
| "db tables" | ||
| "db tables", | ||
| "db size" |
|
Updated with changes (--tables, --size_format) |
danielbachhuber
left a comment
danielbachhuber
left a comment
There was a problem hiding this comment.
Looking good! Some minor comments to address
| * | ||
| * $ wp db size | ||
| * +-------------------+------+---------+ | ||
| * | Name | Size | Bytes | |
There was a problem hiding this comment.
Is it necessary to keep the "Bytes" column if you have the "Size" column?
There was a problem hiding this comment.
No, I can get that removed.
| } | ||
|
|
||
| /** | ||
| * Display the database name and size as ASCII table. |
There was a problem hiding this comment.
Because you can also display the database name and size in other formats too, does it make sense to explicitly describe this as "ASCII table?
| /** | ||
| * Display the database name and size as ASCII table. | ||
| * | ||
| * Displays the Display the database name and size as ASCII table for `DB_NAME` specified in wp-config.php. |
There was a problem hiding this comment.
"Displays the Display the database name" seems like a typo.
In this extended description, it'd be worthwhile to mention the default size format (which is configurable).
| $format = WP_CLI\Utils\get_flag_value( $assoc_args, 'format' ); | ||
| $size_format = WP_CLI\Utils\get_flag_value( $assoc_args, 'size_format' ); | ||
|
|
||
| // For PHP 5.3 compat. |
There was a problem hiding this comment.
This comment seems unnecessary, because it's not clear what was added for 5.3 compat (vs. what was removed)
| * 6 | ||
| */ | ||
| public function size( $args, $assoc_args ) { | ||
|
|
There was a problem hiding this comment.
If I provide --scope=<scope>, --network, --all-tables-with-prefix or --all-tables without the --tables flag, what should happen?
There was a problem hiding this comment.
Went with the same flags and docblock as the tables command, but I can add sample output if you'd like.
|
@petenelson Happy with this? Ready to merge it? |
|
It looks like tests still fail intermittently because the database size changes between command executions :( I'd suggest we can drop the explicit size assertions. I can't think of a good workaround at this point, and it's not worth blocking the pull request. |
|
That was one of the things I was worried would happen with MySQL doing things in the background. I'll get the tests updated shortly. |
danielbachhuber
added
command:db
|
👌 Thanks for your work on this, @petenelson |
|
You're welcome, thanks for entrusting me with is change! |
Adds
db sizecommand, raised in #1